Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web radio chunked demo #48

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

wmarkow
Copy link

@wmarkow wmarkow commented Mar 21, 2020

A simple example how to handle the audio stream with chunked transfer, to avoid sound glitches.
More discussion also in #47.

}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you able to exclude this file from the patchset?
It looks like it's some accidental change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is possible. I did this PR from my presonal branch and this change is there accidentaly.
I will cancel this PR and provide a new one.

Copy link
Owner

@baldram baldram Mar 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need, you can just replace this line from git history or please keep it as it is, and I will exclude it while merging.


uint8_t writeindex = 0;
uint8_t index = 0;
for (index = 0; index < lengt; index++) {
Copy link
Owner

@baldram baldram Mar 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philippedc pointed out that there is a bug: length instead of lengt.
He also said that: "by the way, if the code gives sound, the quality is awful with much more glitches than ever".
I wonder whether it's due to reducing the buffer implementation or something was changed in the code omitting glitches?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my bad. I have fixed the typo in the lengt method signature only. Sorry.

I have no idea why it doesn;t work for @philippedc. For me it works perfect, no more glitches. I will check that again.

Copy link

@philippedc philippedc Mar 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is very strange...
May be your code well suppress the glitches, but the sound quality become so poor it is very brassy. May be it is a matter of performance more than a algorithm issue... by the way it is the kind of code I do not understand how it works...

I have : Arduino IDE last release 1.8.12, core ESP8266 2.4.2 and 2.6.3 (my web radio doesn't compile with 2.6.3 due to the web server library conflict)
IwIP variant : v2 higher bandwidth
CPU : 80MHz or 160MHz

Here are 2 stations that use to glitch a lot. In fact they send many chunk information....
Can you test them with your code ?
23.82.11.89:30228/stream
23.82.11.88:5128/stream

Copy link
Author

@wmarkow wmarkow Mar 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philppedc,

Can you test them with your code ?
23.82.11.89:30228/stream
23.82.11.88:5128/stream

Those two URLs work nice with my code. There is no glitches at all, the sound plays nice. I have used the same settings as you:

core ESP8266 2.4.2
IwIP variant : v2 higher bandwidth
CPU : 80MHz

Does your ESP8266 also hosts a webserver inside? Maybe it has also some impact, so the sound has glitches and is brassy, because the processor also need to do the things related to the webserver?
My test code is simple; I do not have any webserver, so it only reads the stream, remove chunks data information and sends bytes to VS1053.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does your ESP8266 also hosts a webserver inside?

I've tested without and with. No special issue with.
I'm wondering if it is not a matter of cutting length: when my code cuts 5 bytes yours cuts 7 - it is what I notice on the console.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is because you analyze the stream in the 32 bytes chunks. When the leading \r\n is somewhere at the end of your 32 bytes chunk and the ending \r\n is somewhere at the beginning of the next 32 bytes chunk, then your algorithm may not cut it correctly.
My solution has an 8 bytes length middle buffer which is shifted constantly and the algorithm looks for those chunk contorl patterns also constantly, so I do not analyse the stream in chunks; the stream is analysed continuously.

I see, in fact if screen capture has been done with a buffer of 32 bytes, it is working identically with a buffer of 2048 bytes.
May be the glitch is due by a chunk at the high end of the buffer. But that does not answer why I find an alone \r\n in the middle of the stream. I do not have many glitches, average of one every 5 to 10 seconds so it is easy to verify the console.
However you're right with the idea of a shift buffer, I will work on it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so the chunk information is under the format 'number_of_bytes'\r\n. I understand that the preliminary \r\n is optional, and not count in the chunk, right ?
So I did the following:
`

int i=0;
int bytesread = BUFFERSIZE;    // = 4096
static int counter = 0;
int chunk;

if( client.available()) {
  while( i < bytesread ) {
    mp3buff[i] = client.read();

    if( i>1 && mp3buff[i] == '\n' && mp3buff[i-1] == '\r' ) {
      chunk = mp3buff[i-2];
        
      Serial.print(i); Serial.print(":"); Serial.print(chunk); Serial.print(":"); Serial.println(counter);
      counter = 0;
    }
    i++;
    counter++;
  }    // end of while

  player.playChunk(mp3buff, bytesread); 
}      // end of client.available
yield();

} // end of else
} // end of loop
`
which means that each time "I" see a \r\n sequence I read the past byte: if i=\n and i-1=\r so i-2=chunk_size.
Here is the result:
image
1st column => i
2nd column => chunk_size
3rd column => value of the byte counter
I well can see the counter indicates 5 every 2 lines: it is the count of the 5 bytes with the sequence \r\nX\r\n, but I do not understand the correlation between this X value that should be the chunk size and the result of the counter. WHat is wrong ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.... or does that mean when a receive a packet of 1346 bytes, only the first 48 are useful for the streaming ???

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've find the way to solve all my problems. see #52
Many thanks for your help !!!!!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so the chunk information is under the format 'number_of_bytes'\r\n. I understand that the preliminary \r\n is optional, and not count in the chunk, right ?

Preliminary \r\n is not optional, I think. The whole format of the stream is a s below:

metadata_about_the_stream\r\n
ZZZZ\r\n
chunk\r\n
ZZZZ\r\n
chunk\r\n
ZZZ\r\n
chunk\r\n
ZZ\r\n
chunk\r\n
Z\r\n
...

where ZZZZ or ZZZ or ZZ or Z is the length of the next chunk data. But attention, it looks like this length is encoded in ASCII HEX (human readable), so if ZZZZ is a765 then the legth of chunks is 0xa765 or if Z is 5 then the chunk length is 0x5 which is exactly five bytes. If you decode a chunk length byte as 56 DEC then according to the ASCII table it is a digit 8, so the chunk length is 8 bytes (not 56 bytes).

If you take a look again at the format above, you will notice a pattern in the stream. All what needs to be cut out is \r\nZZZZ\r\n or \r\nZZZ\r\n or \r\nZZ\r\n or \r\nZ\r\n, because these are the chunk control data; everything else what is left is an audio stream data.

@CelliesProjects
Copy link

CelliesProjects commented Jun 9, 2021

I wrote a wrapper for this issue. Check out https://github.com/CelliesProjects/ESP32_VS1053_Stream.

With the lib it is as easy as stream.connecttohost("http://icecast.omroep.nl/radio6-bb-mp3"); to open a stream (chunked, https or just plain http) and a loop() function that has to be called every few ms. See #65.

@baldram
Copy link
Owner

baldram commented Jun 9, 2021

Thank you @CelliesProjects for providing a solution to the issue and your work.
I had a look to https://github.com/CelliesProjects/ESP32_VS1053_Stream, next to https://github.com/CelliesProjects/eStreamPlayer32_VS1053 and I must say I'm very impressed.

Now I'm wondering how about those two MRs:

I haven't figured out if we have a clear answer: this works for all people reporting "glitches problem" in this and related issues.
Probably it was still not enough for all cases, without firmware patch, etc.

Are those still valuable examples?
That we can we merge both as a "simple example solution" (with small amount of code)?
Then in both we would add a code comment pointing to ESP32_VS1053_Stream and eStreamPlayer32_VS1053 example,
that if someone wants to do more, he/she might try the more comprehensive solution?

What do you think @CelliesProjects ?
What is your opinion @wmarkow and @fabitom?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants